feat(session): navigate to comments by id - #845
Conversation
|
PR author is not in the allowed authors list. |
|
@Yuki9814 is attempting to deploy a commit to the Modem Team on Vercel. A member of the Team first needs to authorize it. |
There was a problem hiding this comment.
Requesting changes because the new comment-ID navigation path currently drops exact-line reveal semantics and its added integration test fails on Linux.
Validation: bun test test/session/cli.test.ts -t "navigate works" (1 failed).
This comment was generated by Pi using GPT-5.6 Sol
benvinegar
left a comment
There was a problem hiding this comment.
Verified the exact-line precedence fix. Comment-ID navigation now reaches the annotated row and returns side/line; the focused unit tests, Linux session integration regression, and typecheck all pass locally. The PR still has a merge conflict with main that must be resolved before merging.
This comment was generated by Pi using GPT-5.6 Sol
Co-authored-by: OpenAI Codex <codex@openai.com>
59aa164 to
6f2bb5c
Compare
benvinegar
left a comment
There was a problem hiding this comment.
Maintainer-rebased this branch onto current main with the contributor's five commits preserved. The only conflict was the recent parseSessionNavigateCommand extraction; I retained that structure and applied the PR's --comment path within it. Focused tests (206), the live session navigation regression, typecheck, lint, formatting, generated-file checks, and dependency checks pass.
This comment was generated by Pi using GPT-5.6 Sol
Problem
hunk session comment list --jsonexposes stable comment IDs, but callers cannot use one of those IDs to focus that exact comment. They must reconstruct a file/line target or use relative next/previous navigation.Closes #490.
Approach
hunk session navigate --comment <id>and reject combinations with the existing navigation selectors;navigate_to_hunkpayload;This stays in the existing session CLI/daemon path. It does not add a new UI socket command or change how comments are stored.
Validation
Tested on macOS arm64:
bun ./scripts/run-test-suite.ts— 3021 passed, 24 platform/tool-gated skips, 0 failed;bun test src/app/cli.test.ts src/session/protocolSchemas.test.ts src/session/agent/cliClient.test.ts src/session/broker/brokerServer.helpers.test.ts src/session/client/capabilities.test.ts src/session/agent/commands.daemon.test.ts src/hunk-review/skillDocument.test.ts scripts/generate-docs.test.ts— 199 passed, 0 failed;oxfmt --check .;oxlint . --deny-warnings;tsc --noEmit;bun ./scripts/generate-docs.ts --check;bun ./scripts/generate-changelog.ts --check;depcruise src packages --config .dependency-cruiser.cjs --ignore-known.The Linux-only live-session CLI integration is included but platform-gated locally; CI can exercise it. There is no visual UI change.
Implemented with OpenAI Codex assistance. The final diff and verification results were reviewed before submission.